Skip to content

fix bash on fedora#150

Merged
Joeavaikath merged 1 commit intomigtools:oadp-devfrom
weshayutin:fedora_bash
Mar 10, 2026
Merged

fix bash on fedora#150
Joeavaikath merged 1 commit intomigtools:oadp-devfrom
weshayutin:fedora_bash

Conversation

@weshayutin
Copy link
Copy Markdown
Contributor

@weshayutin weshayutin commented Mar 10, 2026 •
edited by coderabbitai bot
Loading

Why the changes were made

fixes: #149

How to test the changes made

whayutin@fedora:~/OPENSHIFT/git/OADP/oadp-cli$ make install
Building kubectl-oadp for current platform (linux/amd64)...
✅ Built kubectl-oadp successfully!
Installing kubectl-oadp to /home/whayutin/.local/bin...
✅ Installed to /home/whayutin/.local/bin

🔍 Checking PATH configuration...
   └─ ✅ /home/whayutin/.local/bin is already in PATH


📋 Configuration:

   🔍 Detecting OADP deployment in cluster...
   ├─ ✅ Found OADP controller in namespace: openshift-adp

   🔍 Looking for DataProtectionApplication (DPA) resources...
   ├─ ✅ Found DPA resource in namespace: openshift-adp

   ├─ Setting Velero namespace to: openshift-adp
   └─ ✅ Client config initialized

🧪 Verifying installation...
   ├─ ✅ Installation verified: kubectl oadp plugin is accessible
   └─ Running version command...

      Client:
      	Version: v0.3.3-44-gdac6316-dirty
      	Git commit: dac6316471354cd005dea3bf4cffe6cfa184aa7f-dirty
      Server:
      	Version: konveyor-dev

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
🎉 Installation complete!

   Quick start:
   • oc oadp --help          # Show available commands
   • oc oadp backup get      # List backups
   • oc oadp version         # Show version info
whayutin@fedora:~/OPENSHIFT/git/OADP/oadp-cli$  oc oadp completion install --shell bash
Installing completions for bash...
✓ Completions files are already installed
Use --force to reinstall

✓ Setup complete! Your ~/.bashrc has bash-completion loaded.

Completions will work after restarting your shell:

Summary by CodeRabbit

  • Bug Fixes
    • Improved bash completion detection across Linux and macOS systems by expanding path checks to include common distribution-specific locations, ensuring more reliable completion installation across different environments.

@weshayutin
fix bash on fedora
b7e747d 
Signed-off-by: Wesley Hayutin <weshayutin@gmail.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 10, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c6131da2-cdb0-4763-b97c-e40be93ee0fb

📥 Commits

Reviewing files that changed from the base of the PR and between dac6316 and b7e747d.

📒 Files selected for processing (1)
  • cmd/completion/install.go
📝 Walkthrough

Walkthrough

The bash completion installation detection logic expands its path checking to include common installation locations across Linux distributions and macOS, removing duplicated entries while maintaining support for existing paths. This improves cross-platform compatibility for identifying bash completion packages.

Changes

Cohort / File(s) Summary
Bash Completion Detection
cmd/completion/install.go		 Extended isBashCompletionInstalled path checks to include standard Linux and macOS locations (/usr/share/bash-completion/bash_completion, /etc/profile.d/bash_completion.sh), removed duplicate path entry, and updated comment for broader distro coverage.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 Hops through paths both far and wide,
Finding bash completion 'cross each side,
Fedora, Ubuntu, macOS too,
No more false alarms, the fix is true! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix bash on fedora' is related to the main change but lacks specificity about what aspect of bash on fedora is being fixed.
Description check ✅ Passed The description follows the template with both required sections completed: 'Why the changes were made' links issue #149, and 'How to test' provides detailed test output demonstrating the fix.
Linked Issues check ✅ Passed The code changes broaden bash completion path checks to include Fedora-specific locations, directly addressing issue #149's requirement to detect bash-completion on Fedora systems.
Out of Scope Changes check ✅ Passed All changes in cmd/completion/install.go are focused on expanding path detection for bash completion and are directly related to fixing the Fedora bash completion issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Joeavaikath Joeavaikath merged commit a6377ad into migtools:oadp-dev Mar 10, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Joeavaikath Joeavaikath Joeavaikath approved these changes

+1 more reviewer

@sseago sseago sseago approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fedora bash completion

3 participants

@weshayutin @sseago @Joeavaikath